@claude review this PR

Claude finished @iamquang95's task in 2m 11s —— View job

Review: feat(app): implement stacksnipe

I compared this against the Charon Go source (charon/app/stacksnipe/stacksnipe.go). This is a clean, faithful, and well-documented port. Tests actually exceed the Go coverage. A few notes below — nothing blocking.

✅ Functional equivalence verified

• Public surface matches Go: Instance, new/new_with_interval, run, snipe, StackComponent, SUPPORTED_VCS, MAYBE_VCS, 15s poll interval.
• NUL-split cmdline → drop empties → join with spaces, comm trimming, dedup-by-raw-cmdline, and substring-based name derivation (from cmdline, not comm) all match the Go logic.
• Empty proc_path disables sniping and logs "Stack component sniping disabled". ✔
• The tracing topic (topic = "stacksnipe") mirrors Go's log.WithTopic. ✔

👍 Good Rust-specific choices

• First-tick skip (stacksnipe.rs:108-111): tokio::time::interval fires immediately on the first tick, whereas Go's time.NewTicker waits one full interval. Skipping the first tick correctly preserves Go's timing, and the comment explains why. Nice catch.
• spawn_blocking (stacksnipe.rs:118): the blocking /proc filesystem walk is correctly moved off the async runtime — better than Go's plain goroutine.
• Making snipe infallible is justified: Go's snipe returns an error, but walkFunc swallows every error and returns nil, so walkErr is effectively never set. filter_map(Result::ok) is the faithful, simpler equivalent.

🟡 Behavioral note — name selection determinism

vc_name (stacksnipe.rs:219-225) returns the first match in SUPPORTED_VCS order via .find(). The Go version iterates a map without break, keeping the last match in non-deterministic iteration order:

for vc := range supportedVCs {
    if strings.Contains(cmdlineString, vc) {
        vcName = vc   // no break → keeps last, map order is random
    }
}

So when a single cmdline contains two supported VC names, Go could report either one (run-to-run), while Rust deterministically reports the first by array order. The Rust behavior is strictly better, but it is a deviation from Go and name_multi_match_deterministic_first bakes it in — worth a one-line comment noting this is an intentional improvement over Go's non-determinism.

🟡 Minor: token vec built before dedup check

In snipe, cli_tokens is allocated at stacksnipe.rs:187 before the dedup check at line 196. Go builds its cmdLine slice after the dedup check, so duplicate cmdlines (e.g. background threads) avoid the per-token String allocations. Reordering — dedup-insert first, then build cli_tokens — would match Go and skip wasted work for duplicates. Purely a micro-optimization. Fix this →

ℹ️ Scope: not yet wired into app startup

Charon wires the sniper into the node lifecycle at app/app.go:170 (stacksnipe.New(conf.ProcDirectory, stackComponents)), where stackComponents writes the Prometheus metric (monitoringapi.go:324). This PR adds the module only — there's no caller of Instance::run and no MetricsFn bridged to a metric. Assuming wiring + the metric are a deliberate follow-up; flagging so it isn't lost.

Tests

Test coverage is thorough and goes beyond Go's single happy-path test (exact-comm-match, name-from-cmdline, dedup, NUL handling, comm newline trim, non-PID dirs, disabled path, nonexistent path). I was unable to execute cargo test in this environment (command not permitted), so please confirm CI is green.

Verdict: Looks good to merge once CI passes. The two 🟡 items are optional polish; the wiring is presumably a follow-up.
· feat/stacksnipe